Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up AZD folder and update core templates #143

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tonybaloney
Copy link
Contributor

This PR removes core files that aren't used and updates the ones that are there from the latest version

@tonybaloney
Copy link
Contributor Author

/home/runner/work/contoso-chat/contoso-chat/infra/main.bicep(76,3) : Error BCP035: The specified "object" declaration is missing the following required properties: "openAiConnectionName".
/home/runner/work/contoso-chat/contoso-chat/infra/main.bicep(84,5) : Error BCP037: The property "appInsightsName" is not allowed on objects of type "params". Permissible properties include "applicationInsightsName", "openAiConnectionName", "searchConnectionName", "searchServiceName".
/home/runner/work/contoso-chat/contoso-chat/infra/main.bicep(94,5) : Error BCP037: The property "searchName" is not allowed on objects of type "params". Permissible properties include "applicationInsightsName", "openAiConnectionName", "searchConnectionName", "searchServiceName".

Those files were probably changed, will investigate

@tonybaloney
Copy link
Contributor Author

This could be consolidated with the role assignments parameter above https://github.com/Azure-Samples/contoso-chat/blame/main/infra/core/database/cosmos/sql/cosmos-sql-db.bicep#L70-L82 have amended the bicep

@@ -90,8 +90,9 @@ module ai 'core/host/ai-environment.bicep' = {
? storageAccountName
: '${abbrs.storageStorageAccounts}${resourceToken}'
openAiName: !empty(openAiName) ? openAiName : 'aoai-${resourceToken}'
openAiConnectionName: 'aoai-connection-${resourceToken}'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nitya is this related to #137?

@tonybaloney tonybaloney marked this pull request as ready for review July 4, 2024 02:04
@tonybaloney
Copy link
Contributor Author

@nitya please can you review

@nitya
Copy link
Contributor

nitya commented Jul 5, 2024

@tonybaloney - missed seeing this earlier. Will review and ping you directly for clarification. Have some existing changes to merge to fix other azd issues so want to make sure these are complementary

@kristenwomack
Copy link

@tonybaloney - missed seeing this earlier. Will review and ping you directly for clarification. Have some existing changes to merge to fix other azd issues so want to make sure these are complementary

@nitya - these are complimentary, and we've discussed these changes.

@jongio
Copy link
Member

jongio commented Aug 7, 2024

@tonybaloney - Can this be moved to avm instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants